fix: enforce @RolesAllowed on microservice resources#5199
Conversation
Re-applies apache#5049 (revert in apache#5173) and additionally marks the two pre-login ConfigResource endpoints (/config/gui, /config/user-system) as @permitAll so the frontend's APP_INITIALIZER can still bootstrap once role enforcement is on. Adds an HTTP-pipeline regression test that fires unauthenticated requests through JwtAuthFilter + RolesAllowedDynamicFeature and asserts the two pre-login endpoints return 200 while a sibling @RolesAllowed probe returns 403. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
dependencies
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5199 +/- ##
============================================
+ Coverage 45.39% 45.50% +0.11%
- Complexity 2218 2223 +5
============================================
Files 1042 1042
Lines 39989 39997 +8
Branches 4260 4260
============================================
+ Hits 18152 18201 +49
+ Misses 20720 20677 -43
- Partials 1117 1119 +2
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@Yicong-Huang I raised a similar pr to fix this. #5198 |
I hope to minimize the change, this PR is reapplying #5049 with only two more line diff about permit all usage on configservice. could you please help test this one? |
That makes sense. I can help test and review later today when I get back home. |
|
Shouldn’t Matthew’s PR have priority? Also this PR is bigger |
|
I believe that this PR focuses on reimplementing the previous PR without its problems, while my PR aims to extend the coverage of @RolesAllowed (to address the issue). I think @Yicong-Huang can merge this one, and I'll rebase mine on top as a complementary PR for now. |
4 participants
What changes were proposed in this PR?
Re-applies #5049 (Jersey
@RolesAllowedenforcement onconfig-service,computing-unit-managing-service, andworkflow-compiling-service) and additionally marks the two pre-loginConfigResourceendpoints —/api/config/guiand/api/config/user-system— as@PermitAll. Those endpoints are loaded byGuiConfigService.load()in the AngularAPP_INITIALIZERbefore any login, so once role enforcement is on they must keep returning200to unauthenticated callers; missing this was what broke bootstrap and got #5049 reverted in #5173. Everything outsideconfig-servicematches #5049 byte-for-byte.Any related issues, documentation, or discussions?
Closes: #4904
Prior attempt: #5049, reverted by #5173. The bootstrap root cause was diagnosed inline at #5049 (comment).
How was this PR tested?
Added
ConfigResourceAuthSpec: wiresConfigResourcethrough the sameJwtAuthFilter+RolesAllowedDynamicFeaturepipeline production uses (via Dropwizard'sResourceExtension) and fires HTTP requests with noAuthorizationheader.GET /config/gui→ expects200GET /config/user-system→ expects200GET /auth-probe(an in-test@RolesAllowedresource) → expects403The
403sanity guard ensures the feature is actually enforcing, so a future "200 everywhere" regression cannot silently slip through. Kept the three*ServiceRunSpecstructural tests from #5049 verifying thatRolesAllowedDynamicFeatureis registered. Manual reproduction withcurlagainst a local dev server confirmed the unauthenticated bootstrap path returns200while a low-role JWT against an annotated endpoint returns403.Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.7 in compliance with ASF.